fix(trampoline): don't set PYTHONHOME for venv trampolines#19123
fix(trampoline): don't set PYTHONHOME for venv trampolines#19123Bojun-Vvibe wants to merge 2 commits intoastral-sh:mainfrom
Conversation
The PYTHONHOME env var was set whenever the resolved `python_exe` was not itself a virtualenv, but for venv trampolines `python_exe` is the base interpreter outside the venv. The check now also inspects the trampoline's own location, so venvs built on managed interpreters no longer leak PYTHONHOME pointing at the base install into child processes (which could mix stdlib across Python versions).
|
This seems vaguely correct, though we may want to remove the |
| // - Override inherited `PYTHONHOME` from parent Python processes | ||
| // - Preserve user-defined `PYTHONHOME` values | ||
| if !is_virtualenv(python_exe.as_path()) { | ||
| // |
There was a problem hiding this comment.
The test-windows-trampolines / check on * CI jobs are failing. Per the workflow, these run cargo fmt --check and cargo clippy --all-features --target x86_64-pc-windows-msvc --tests -- -D warnings.
Please run:
cd crates/uv-trampoline
cargo fmt
cargo clippy --all-features --target x86_64-pc-windows-msvc --tests -- -D warnings
Then push the formatting fixes.
|
I'm taking a deeper look at this and I do want to try to unset PYTHONHOME in more cases. I may end up opening a separate PR for that. |
Signed-off-by: Bojun Chai <bojunchai@microsoft.com>
|
Thanks for the pointer! Pushed |
|
So the issue with this (which I admit I had to have Claude remind me of, though I should have suspected it myself) is that it doesn't work right on Python 3.10 and below. The older C-based implementation of getpath used up to 3.10 gets confused when you set However, you don't actually need to set |
|
Thanks for the careful read and for catching the 3.10 getpath behavior — your conditional-on-venv approach in #19199 is cleaner and avoids the |
|
Superseded by #19199 (better approach). |
5 participants
Closes #19080
Repo
astral-sh/uv
Issue
#19080
Root cause
The Windows trampoline in
crates/uv-trampoline/src/bounce.rsdecides whether toset
PYTHONHOMEby callingis_virtualenv(python_exe). For a venv trampoline,python_exeresolves to the base managed interpreter (e.g.~\AppData\Roaming\uv\python\cpython-3.13-...\python.exe), which is not itselfa virtualenv. So even when the user invokes
.venv\Scripts\python(which IS avenv trampoline),
PYTHONHOMEis set to the base install. That env var thenleaks to child processes (e.g. cibuildwheel running a different Python),
shadowing the wrong stdlib and producing
AttributeError: module '_thread' has no attribute 'start_joinable_thread'.Fix
Also check
is_virtualenv(executable_name)— the trampoline's own path — beforedeciding to set
PYTHONHOME. A venv trampoline lives in<venv>\Scripts\nextto a
<venv>\pyvenv.cfg, so this correctly suppressesPYTHONHOMEfor venvtrampolines while preserving the original junction-handling behavior for
non-venv managed interpreters.
Regression test
None added. The trampoline crate (
crates/uv-trampoline) is excluded from theworkspace, builds only as a Windows binary with
panic-immediate-abort/#![no_main], and has no existing unit-test surface. The patch is atwo-line guard change adjacent to the existing
is_virtualenv(python_exe)check and follows the same pattern.
Risk
low
Verification
skipped: build requires Windows MSVC toolchain (cross-check on macOS fails on
rust-srcfor the nightly toolchain). The change is a localized predicateextension reusing the existing
is_virtualenvhelper.